Skip to content

Added support for Hijack UDP#83

Open
axkum10 wants to merge 2 commits intoPlatformLab:mainfrom
axkum10:main
Open

Added support for Hijack UDP#83
axkum10 wants to merge 2 commits intoPlatformLab:mainfrom
axkum10:main

Conversation

@axkum10
Copy link
Copy Markdown

@axkum10 axkum10 commented Apr 23, 2026

Hello John,

The code change is for supporting hijack UDP. UDP will be slower than TCP, due to absence of TSO.
Where UDP can be used:
1. When network blocks unknown protocol
2. When network blocks TCP (due to tcp state machine or firewall preventing TCP flag "SR" to pass through).

Thanks

axkum10 added 2 commits April 22, 2026 22:24
	modified:   homa_hijack.h
	modified:   homa_impl.h
	modified:   homa_outgoing.c
	modified:   homa_plumbing.c
	modified:   homa_qdisc.c
	modified:   homa_wire.h
	modified:   util/cp_node.cc
	modified:   util/homa_test.cc
	modified:   util/server.cc
Copy link
Copy Markdown
Member

@johnousterhout johnousterhout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some overall comments:

  • The UDP hijacking triggers don't seem safe to me. In order to determine whether an incoming UDP packet actually belongs to Homa, homa_skb_udp_hijacked is testing bits that fall in the data portion of the packet. This means that a real UDP packet could contain data that causes Homa to steal the packet incorrectly.

  • The UDP header fields (length and checksum) conflict with TCP header fields (and Homa's offset in data packets), so I think you would need to define a different Homa packet format for UDP hijacking; I'm a bit nervous that this will create a lot of additional complexity.

  • How likely do you think it is that UDP hijacking will be needed for Mastercard's Homa deployment? My understanding from previous experiments was that Homa packets can successfully be transmitted across Mastercard's datacenter; have I misremembered this? And, if native Homa packets can be used, is there any advantage to UDP hijacking?

  • The cp_node support for UDP is problematic because UDP doesn't provide reliable delivery. Won't a single lost packet cause tests to hang? One possibility would be to implement timeout and retry for UDP, but overall this feels like a big slug of additional code and I'm not sure that it adds enough value to justify the support burden it will create. People naturally want to compare Homa to TCP, so it makes sense to have TCP support in cp_node, but will there be similar interest for UDP (no-one has asked me for Homa-UDP comparisons yet)? If you don't have a significant purpose in mind for the code, I'm inclined to leave it out. The code in homa_test.cc and server.cc is smaller, and I can imagine it might work well enough for some simple performance tests, so I don't object to that.

  • All code in the top-level Homa directory needs to have unit tests. Can you add new tests for the code you have added? They are in the test subdirectory. I just created a README.md file for that directory, but I'm not currently in a position where I can push it; I have cut and pasted the contents below. Let me know if you have questions or run into problems.

Contents of test/README.md:

This directory contains unit tests for the Linux kernel implementation
of Homa.

  • To run the tests, invoke "make test" in this directory. This will
    compile a binary named unit and run it. You can run unit under
    gdb for debugging.

  • The tests have a structure that is isomorphic to the code (this makes it
    easier to see that coverage is complete and determine whether new tests
    need to be added when making code changes):

    • There is one test file for each .c file in the parent directory;
      for a code file homa_foo.c, the test file will be unit_homa_foo.c.
    • Within the test file, there is a group of tests for each function
      in the code file, in the same order as the functions in the code
      file.
    • The tests for a given function should validate the features of the code
      in that function (e.g. conditionals, loops, etc.). Each test should
      focus on a single feature or a small number of related features.
      The order of tests for a function should match the order of the
      features within the code.
    • If a header file contains nontrivial inline functions, unit tests
      for it should appear at the end of the test file used for the
      corresponding .c file.
  • The name for a test in the TEST_F line should have the form
    func__feature, where func is the name of the function and
    feature describes the thing being tested (multiple words separated
    by underscores).

  • It's not unusual to start the tests for a given function with a
    basic sanity check with name func__basics. This may exercise many
    features of the function; the remaining tests only need to validate
    things not checked by the __basic test.

  • In addition to the unit tests, this file contains several additional
    files that provide support for unit tests:

    • mock.c: stubs out numerous Linux kernel functions and features so
      that tests can be run outside the kernel. This file also contains a
      few helper functions such as mock_skb_alloc: check for function
      names that start with mock_.
    • unit.c: C functions that help in writing tests, such as
      unit_client_rpc and unit_server_rpc for creating RPC structures
      to use in tests.
    • ccutils.c: functions written in C++, but with C interfaces, to help in
      writing tests. For example, this includes functions to manage hash tables
      using C++ std::unordered_map objects and functions to manage a test
      log using C++ strings.
      *kselftest_harness.h: this defines the testing infrastructure; it is
      a modified version of the kernel file
      tools/testing/selftests/kselftest_harness.h.

Comment thread homa_hijack.c
* homa_hijack.h for an overview of TCP hijacking.
/* This file implements TCP and UDP hijacking for Homa. See comments at the
* top of homa_hijack.h for an overview of TCP hijacking. UDP hijacking works
* similarly but uses UDP as the IP protocol, which avoids issues with
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding this sentence here, rework the comment at the beginning of homa_hijack.h so that it describes both TCP and UDP hijacking in a coordinated way.

Comment thread homa_hijack.c
* @skb: The newly arrived packet.
*/
struct sk_buff *homa_udp_hijack_gro_receive(struct list_head *held_list,
struct sk_buff *skb)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linux style guidelines require the 's' in 'struct' to align under the 's' in 'struct' of the preceding line.

Comment thread homa_hijack.h
transport_len = skb->len - skb_transport_offset(skb);

/* Set UDP length at bytes 4-5 (overlaps high 16 bits of sequence). */
*((__be16 *)((u8 *)h + 4)) = htons(transport_len);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is too cryptic; please use C structure definitions to manage the UDP header fields (e.g. all the info needed for UDP hijacking should be clearly visible, symbolically, in homa_wire.h).

Comment thread homa_hijack.h
struct sk_buff *
homa_udp_hijack_gro_receive(struct list_head *held_list,
struct sk_buff *skb);
void homa_udp_hijack_init(void);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functions should be listed in alphabetical order.

Comment thread homa_outgoing.c
if (homa_sock_udp_hijacked(rpc->hsk))
homa_udp_hijack_set_hdr(skb, rpc->peer, false);
else
homa_hijack_set_hdr(skb, rpc->peer, false);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a lot of places where code now has to do separate checks for TCP and UDP hijacking. Is there a way to reorganize things so that only a single check is needed?

Comment thread homa_plumbing.c
.maxlen = sizeof(int),
.mode = 0644,
.proc_handler = homa_dointvec
},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a separate parameter 'hijack_udp' feels awkward to me: what happens if someone sets both 'hijack_tcp' and 'hijack_udp'? I think it would be better to change things to have a single parameter 'hijack' with three values: none, TCP, or UDP.

@johnousterhout
Copy link
Copy Markdown
Member

BTW, I just got access to an AWS machine running RHEL 8.9, and I see that the unit tests are pretty badly broken on that version. I'm in the process of fixing them, so I would suggest that you don't worry about unit tests until I have a change to get them working properly on the rhel8 branch.

@johnousterhout
Copy link
Copy Markdown
Member

The unit tests now run for me in the rhel8 branch; I just pushed the changes to GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants